Support proto2 groups #640

timostamm · 2023-12-04T13:38:48Z

This PR implements support for the proto2 groups, which allows to declare a field and a message at the same time.

Here is an example - message M has a group field g:

syntax="proto2";
message M {
  optional group G = 1 {
    string field = 1;
  }
}

In descriptors, groups are represented as messages, and like many other implementations, Protobuf-ES generates groups as messages, Unfortunately, it does not implement the group wire format, and fails to serialize / parse messages correctly from the binary format.

This PR fixes the issue. It adds the property delimited to the FieldInfo type, and sets it to true in generated code for group fields (it is false or undefined otherwise). Based on this information, group fields are serialized and parsed with the proper tag-delimited format.

timostamm · 2023-12-04T14:04:51Z

packages/protobuf/src/binary-format.ts

+   *
+   * Supports two message encodings:
+   * - length-prefixed: delimitedMessageEncoding is false or omitted, and
+   *   lengthOrEndTagFieldNo is the expected length of the message in the reader.
+   * - delimited: delimitedMessageEncoding is true, and lengthOrEndTagFieldNo is
+   *   the field number in a tag with wire type end-group signalling the end of
+   *   the message in the reader.
+   *
+   * delimitedMessageEncoding is optional for backwards compatibility.
   */
  readMessage(
    message: Message,
    reader: IBinaryReader,
-    length: number,
+    lengthOrEndTagFieldNo: number,
    options: BinaryReadOptions,
+    delimitedMessageEncoding?: boolean,


This method is part of the public API. The new signature is backwards-compatible. It's not pretty, but this way we can avoid looking ahead for the end group tag.

jhump

Couple of questions, but looks great to me!

packages/protobuf/src/private/binary-format-common.ts

@smaye81

## What's Changed * Add explicit exports for Node by @smaye81 in #645 * Honor experimental editions in the preamble generated by `@bufbuild/protoplugin` by @timostamm in #644 * Fix typing error in `@bufbuild/protoplugin`, improve error messages by @timostamm in #642 * Support for proto2 groups by @timostamm in #640 **Full Changelog**: v1.5.0...v1.5.1

Support proto2 groups

dfd5a61

timostamm commented Dec 4, 2023

View reviewed changes

timostamm added 2 commits December 4, 2023 15:32

Make new FieldInfo property optional to avoid BC

906a40f

Update test file to use one consistent import style

a080109

timostamm mentioned this pull request Dec 4, 2023

Disable deprecated proto2 groups #630

Closed

timostamm requested review from smaye81 and jhump December 4, 2023 14:37

timostamm marked this pull request as ready for review December 4, 2023 14:37

jhump approved these changes Dec 4, 2023

View reviewed changes

packages/protobuf/src/private/binary-format-common.ts Show resolved Hide resolved

packages/protobuf/src/private/binary-format-common.ts Show resolved Hide resolved

timostamm merged commit ce54cf1 into main Dec 5, 2023
5 checks passed

timostamm deleted the tstamm/support-proto2-groups branch December 5, 2023 10:27

smaye81 mentioned this pull request Dec 6, 2023

Release 1.5.1 #647

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support proto2 groups #640

Support proto2 groups #640

timostamm commented Dec 4, 2023 •

edited

Loading

timostamm Dec 4, 2023

jhump left a comment

Support proto2 groups #640

Support proto2 groups #640

Conversation

timostamm commented Dec 4, 2023 • edited Loading

timostamm Dec 4, 2023

Choose a reason for hiding this comment

jhump left a comment

Choose a reason for hiding this comment

timostamm commented Dec 4, 2023 •

edited

Loading